update updates to use propel orm #6867
Conversation
DAcodedBEAT
force-pushed
the
propel-for-updates
branch
from
c5027c3 to
6ffe7a7
Compare
DAcodedBEAT
added
Code Smell
php
|
Looks good so far |
|
Ya that sounds good |
| ->setText(InputUtils::filterHTML($sEventText)) | ||
| ->setStart(InputUtils::legacyFilterInput($sEventStart)) | ||
| ->setEnd(InputUtils::legacyFilterInput($sEventEnd)) | ||
| ->setInActive(InputUtils::legacyFilterInput($iEventStatus)); |
There was a problem hiding this comment.
less important, but it seems in this case it should be setInactive, as the 'In' is not a fully separate word.
There was a problem hiding this comment.
I agree, just using what was already set up to not scope creep - https://github.com/ChurchCRM/CRM/blob/master/propel/schema.xml#L506
There was a problem hiding this comment.
If we wanted to open an Issue to potentially change the schema, to have consistent camelCase .... should we consider an additional TEMPLATE type in ISSUE_TEMPLATES? Maybe something like CodeCleanup? This wouldn't really be a bug, though I guess it could be argued to be a feature -- even though it shouldn't have any impact.
Also, do you know offhand if updating the schema (with name-format changes) would have any ripple effects? Or should everything just be properly auto-generated from that? And I haven't tried to find out what actually does that code generation, but it looks like all the files in the model/ChurchCRM/Base dir are being generated, so hopefully just changing schema is adequate.
DAcodedBEAT
force-pushed
the
propel-for-updates
branch
from
e79a3b0 to
d3d7d23
Compare
DAcodedBEAT
changed the title
Description & Issue number it closes
Screenshots (if appropriate)
How to test the changes?
Type of change
How Has This Been Tested?
Checklist: